fix: the output is a contract, so document it and version it (3.6.1) - #13
Conversation
Second developer review, run under the 3.6.0 stop rule: look for a new class, not another instance. Nine adversarial probes found every closed class still closed. Pipe and hash in filenames cannot forge table columns or headings, a symlink to ~/.ssh leaks nothing, binary and invalid-UTF-8 files do not crash it, a 400k-line file hits the byte cap, a symlink loop does not hang, 10k files take ~10s, and two runs on the same commit are byte-identical in markdown and JSON. One new class: the output is read by the skill and by users' CI, and it was neither documented nor versioned. - Exit codes 0/1/2 documented in --help and SKILL.md. 3.5.2 added exit 2 and documented it nowhere. Exit 2 is never a verdict about the code. - JSON carries schema_version, currently 1. Bumps only when a field is removed or changes meaning. - --context is flattened through _one_safe_line, so it can no longer render as a heading or forge table columns. Same shape as the waiver injection fixed in 3.4.0, lower severity because the input is the operator's own typing, and the helper already existed. - README no longer implies the whole toolkit is tested. 83 of 83 is a number about prod-readiness-coach, one of six tools and the only one that is a script. The other five are prompts, hand-tested once. Saying it without that sentence was this toolkit overstating what it checked. 118 tests.
|
Warning Review limit reached
Next review available in: 22 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Second developer review, run under the stop rule from 3.6.0: look for a new class of defect, not another instance of a closed one.
Every closed class stayed closed
I did not re-run the 0-to-10 DX rubric. That lens can never return "nothing found." I tried to break the tool instead.
|to forge a table column|#to forge a heading~/.ssh/id_ed25519and/etc/passwdThat is a pass. The stop rule worked.
One new class: the output is a contract
Nobody had looked at the tool as an API. Three instances.
1. Exit codes were undocumented, and 3.5.2 changed them.
That release added exit 2 and said so nowhere.
--helplisted none of the three. Anyone wiring CI had to read the source.Verified: missing path → 2, criticals present → 1, no gate → 0.
2. The JSON had no
schema_version.SKILL.mdtells Claude to parsecategories[].checks[].id, and 3.5.2 addedis_git_repoto the fingerprint. A consumer could not tell which shape it held. Now"schema_version": 1, bumped only when a field is removed or changes meaning. Adding a field is not a break.3.
--contextcould forge report structure.rendered as a heading inside the quote block. Now flattened by
_one_safe_line(), the same helper that has sanitized the waiver file since 3.4.0:Same shape as the waiver injection fixed at HIGH in 3.4.0. Lower severity here, because a waiver file comes from the repository and a
--contextstring comes from the person running the command. The helper already existed.And a finding against the last release
The README said "83 of 83" with no qualifier. I wrote that line two hours ago and it was wrong.
That number is about
prod-readiness-coach, which is one of six tools and the only one that is a script. The other five (clean-code-review,boy-scout-cleanup,clean-code-scaffold,developer-handoff,product-readiness-review) are prompts, tested once by hand, with nothing that runs twice the same way.Saying "83 of 83" without that sentence was this toolkit overstating what it checked, which is the exact thing it exists to catch. The README now says which tool the number covers and states plainly that the other five carry weaker evidence.
(Why those five have no automated tests:
claude plugin evalis gated behind early access on this account. Not fixable here.)Test plan
coverage_grid.py --fail-under 100exits 0./scripts/validate-toolkit.shgreen--fail-on criticalon this repo, exit 0plugin.jsonandmarketplace.json(x2), CHANGELOG entry